apps/ui/src/electron/utils/icon-manager.ts (1)

19-33: Use app root instead of __dirname for icon path resolution

__dirname can shift between dev and packaged builds, which makes ../../dist/public fragile. Prefer anchoring to app.getAppPath() so the path is tied to the app root and less sensitive to bundling layout changes. Please confirm the packaged layout still matches dist/public.

♻️ Proposed refactor

-  const iconPath = isDev
-    ? path.join(__dirname, '../../public', iconFile)
-    : path.join(__dirname, '../../dist/public', iconFile);
+  const appPath = app.getAppPath();
+  const iconPath = isDev
+    ? path.join(appPath, 'public', iconFile)
+    : path.join(appPath, 'dist', 'public', iconFile);

apps/ui/src/electron/ipc/window-handlers.ts (1)

15-23: Consider removing the unused sidebarExpanded parameter.

The _sidebarExpanded parameter is accepted but not used since the logic now uses a fixed minimum width. If this API contract is no longer needed, removing the parameter would simplify both the handler and the preload API.

♻️ Optional: Remove unused parameter

If backward compatibility isn't required:

-  ipcMain.handle(IPC_CHANNELS.WINDOW.UPDATE_MIN_WIDTH, (_, _sidebarExpanded: boolean) => {
+  ipcMain.handle(IPC_CHANNELS.WINDOW.UPDATE_MIN_WIDTH, () => {

And update preload.ts:

-  updateMinWidth: (sidebarExpanded: boolean): Promise<void> =>
-    ipcRenderer.invoke(IPC_CHANNELS.WINDOW.UPDATE_MIN_WIDTH, sidebarExpanded),
+  updateMinWidth: (): Promise<void> =>
+    ipcRenderer.invoke(IPC_CHANNELS.WINDOW.UPDATE_MIN_WIDTH),

apps/ui/src/electron/ipc/shell-handlers.ts (1)

14-22: Consider validating URL scheme before opening external links.

shell.openExternal() can execute arbitrary protocol handlers, which poses a security risk if the renderer process is compromised. Consider restricting to safe schemes.

🔒 Suggested validation

   ipcMain.handle(IPC_CHANNELS.SHELL.OPEN_EXTERNAL, async (_, url: string) => {
     try {
+      const parsedUrl = new URL(url);
+      const allowedSchemes = ['http:', 'https:', 'mailto:'];
+      if (!allowedSchemes.includes(parsedUrl.protocol)) {
+        return { success: false, error: `Blocked URL scheme: ${parsedUrl.protocol}` };
+      }
       await shell.openExternal(url);
       return { success: true };

apps/ui/src/electron/windows/main-window.ts (1)

63-71: Simplify redundant URL loading branches.

The isDev fallback (lines 66-68) and production branch (lines 69-71) load the same URL. This can be consolidated.

♻️ Suggested simplification

   // Load Vite dev server in development or static server in production
   if (VITE_DEV_SERVER_URL) {
     state.mainWindow.loadURL(VITE_DEV_SERVER_URL);
-  } else if (isDev) {
-    // Fallback for dev without Vite server URL
-    state.mainWindow.loadURL(`http://localhost:${state.staticPort}`);
   } else {
+    // Static server URL (dev fallback or production)
     state.mainWindow.loadURL(`http://localhost:${state.staticPort}`);
   }

apps/ui/src/electron/windows/window-bounds.ts (1)

110-129: Apply minimum size even when recentering off-screen.

The off-screen branch returns width/height without MIN_* enforcement; clamping here keeps behavior consistent and guards against corrupted bounds.

♻️ Suggested refinement

-    return {
-      x: x + Math.floor((width - bounds.width) / 2),
-      y: y + Math.floor((height - bounds.height) / 2),
-      width: Math.min(bounds.width, width),
-      height: Math.min(bounds.height, height),
-      isMaximized: bounds.isMaximized,
-    };
+    const clampedWidth = Math.max(Math.min(bounds.width, width), MIN_WIDTH_COLLAPSED);
+    const clampedHeight = Math.max(Math.min(bounds.height, height), MIN_HEIGHT);
+    return {
+      x: x + Math.floor((width - clampedWidth) / 2),
+      y: y + Math.floor((height - clampedHeight) / 2),
+      width: clampedWidth,
+      height: clampedHeight,
+      isMaximized: Boolean(bounds.isMaximized),
+    };

apps/ui/src/electron/server/backend-server.ts (3)

66-88: Simplify redundant tsx resolution fallback logic.

The nested try-catch structure is confusing and contains duplicate fallback code (lines 72-78 and 81-87 are identical). The outer catch at line 80 will catch any exception from the inner block, making the inner catch at line 76 redundant for recovery purposes.

♻️ Suggested simplification

-    let tsxCliPath: string;
-    // Check for tsx in app bundle paths
-    try {
-      if (electronAppExists(path.join(serverNodeModules, 'dist/cli.mjs'))) {
-        tsxCliPath = path.join(serverNodeModules, 'dist/cli.mjs');
-      } else if (electronAppExists(path.join(rootNodeModules, 'dist/cli.mjs'))) {
-        tsxCliPath = path.join(rootNodeModules, 'dist/cli.mjs');
-      } else {
-        try {
-          tsxCliPath = require.resolve('tsx/cli.mjs', {
-            paths: [path.join(__dirname, '../../../server')],
-          });
-        } catch {
-          throw new Error("Could not find tsx. Please run 'npm install' in the server directory.");
-        }
-      }
-    } catch {
-      try {
-        tsxCliPath = require.resolve('tsx/cli.mjs', {
-          paths: [path.join(__dirname, '../../../server')],
-        });
-      } catch {
-        throw new Error("Could not find tsx. Please run 'npm install' in the server directory.");
-      }
-    }
+    let tsxCliPath: string | undefined;
+    // Check for tsx in app bundle paths
+    const serverTsxPath = path.join(serverNodeModules, 'dist/cli.mjs');
+    const rootTsxPath = path.join(rootNodeModules, 'dist/cli.mjs');
+    
+    try {
+      if (electronAppExists(serverTsxPath)) {
+        tsxCliPath = serverTsxPath;
+      } else if (electronAppExists(rootTsxPath)) {
+        tsxCliPath = rootTsxPath;
+      }
+    } catch {
+      // electronAppExists failed, fall through to require.resolve
+    }
+    
+    if (!tsxCliPath) {
+      try {
+        tsxCliPath = require.resolve('tsx/cli.mjs', {
+          paths: [path.join(__dirname, '../../../server')],
+        });
+      } catch {
+        throw new Error("Could not find tsx. Please run 'npm install' in the server directory.");
+      }
+    }

---

95-101: Preserve original error context when server path validation fails.

The catch block swallows any exception from electronAppExists and throws a generic message, losing the original error context (e.g., permission denied). This is inconsistent with the error handling pattern at lines 44-48.

♻️ Suggested fix

     try {
       if (!electronAppExists(serverPath)) {
         throw new Error(`Server not found at: ${serverPath}`);
       }
-    } catch {
-      throw new Error(`Server not found at: ${serverPath}`);
+    } catch (error) {
+      const originalError = error instanceof Error ? error.message : String(error);
+      throw new Error(`Server not found at: ${serverPath}. Reason: ${originalError}`);
     }

---

139-139: Non-null assertion on state.apiKey may cause silent failures.

Using state.apiKey! assumes ensureApiKey() was called successfully before startServer(). If apiKey is null/undefined, the server would receive undefined as the AUTOMAKER_API_KEY environment variable, potentially causing authentication issues that are hard to debug.

Consider adding a guard or fallback:

♻️ Suggested fix

     // Pass API key to server for CSRF protection
-    AUTOMAKER_API_KEY: state.apiKey!,
+    AUTOMAKER_API_KEY: state.apiKey ?? '',

Or add an explicit check earlier:

if (!state.apiKey) {
  throw new Error('API key not initialized. Call ensureApiKey() before starting server.');
}

apps/ui/src/electron/server/backend-server.ts (1)

70-90: Redundant fallback logic in tsx path resolution.

The outer catch block (lines 81-90) duplicates the require.resolve fallback already present in lines 77-79. If electronAppExists throws (which is unlikely given it's a simple existence check), both paths attempt the same resolution.

♻️ Simplified flow

     let tsxCliPath: string;
-    // Check for tsx in app bundle paths, fallback to require.resolve
     const serverTsxPath = path.join(serverNodeModules, 'dist/cli.mjs');
     const rootTsxPath = path.join(rootNodeModules, 'dist/cli.mjs');

     try {
       if (electronAppExists(serverTsxPath)) {
         tsxCliPath = serverTsxPath;
       } else if (electronAppExists(rootTsxPath)) {
         tsxCliPath = rootTsxPath;
       } else {
-        // Fallback to require.resolve
         tsxCliPath = require.resolve('tsx/cli.mjs', {
           paths: [path.join(__dirname, '../../server')],
         });
       }
-    } catch {
-      // electronAppExists threw or require.resolve failed
-      try {
-        tsxCliPath = require.resolve('tsx/cli.mjs', {
-          paths: [path.join(__dirname, '../../server')],
-        });
-      } catch {
+    } catch {
         throw new Error("Could not find tsx. Please run 'npm install' in the server directory.");
-      }
     }

apps/ui/src/electron/server/static-server.ts (3)

62-80: Consider handling the case where index.html itself fails to load.

If the initial file isn't found (line 64 falls back to index.html), but then reading index.html also fails, the server returns a generic 500 error. This is acceptable, but logging the error would aid debugging in production.

Additionally, the nested callback pattern works but could be simplified with promisified versions if electronAppStat/electronAppReadFile support them.

---

83-89: Error listener is added but never removed.

The 'error' listener on line 88 is added but not cleaned up if an error occurs. While this doesn't cause issues during normal operation, if startStaticServer is called again after a failed attempt, the old listener remains. Consider removing the listener after resolution or rejection.

♻️ Suggested improvement

   return new Promise((resolve, reject) => {
+    const onError = (err: Error) => reject(err);
     state.staticServer!.listen(state.staticPort, () => {
+      state.staticServer!.removeListener('error', onError);
       logger.info('Static server running at http://localhost:' + state.staticPort);
       resolve();
     });
-    state.staticServer!.on('error', reject);
+    state.staticServer!.on('error', onError);
   });

---

95-101: stopStaticServer does not wait for close to complete.

server.close() is asynchronous. If you need to ensure the server is fully stopped before proceeding (e.g., during app quit or restart), consider returning a Promise or accepting a callback.

♻️ Optional async version

-export function stopStaticServer(): void {
+export async function stopStaticServer(): Promise<void> {
   if (state.staticServer) {
     logger.info('Stopping static server...');
-    state.staticServer.close();
-    state.staticServer = null;
+    await new Promise<void>((resolve) => {
+      state.staticServer!.close(() => resolve());
+    });
+    state.staticServer = null;
   }
 }

apps/ui/src/electron/server/backend-server.ts (1)

167-175: Process event handlers don't prevent multiple firings.

Both 'close' and 'error' events set state.serverProcess = null. If both events fire (which can happen), the second handler will execute on a null reference. While currently benign, this could mask issues.

♻️ Guard against multiple firings

   state.serverProcess.on('close', (code) => {
+    if (!state.serverProcess) return;
     serverLogger.info('Process exited with code', code);
     state.serverProcess = null;
   });

   state.serverProcess.on('error', (err) => {
+    if (!state.serverProcess) return;
     serverLogger.error('Failed to start server process:', err);
     state.serverProcess = null;
   });

apps/ui/src/electron/utils/icon-manager.ts (1)

22-28: Simplify platform branching.

The darwin and else branches are identical. Consider consolidating them for clarity.

♻️ Suggested simplification

   let iconFile: string;
   if (process.platform === 'win32') {
     iconFile = 'icon.ico';
-  } else if (process.platform === 'darwin') {
-    iconFile = 'logo_larger.png';
   } else {
+    // macOS, Linux, and others use PNG
     iconFile = 'logo_larger.png';
   }

apps/ui/src/electron/windows/main-window.ts (1)

64-72: Redundant conditional branches.

Lines 67-71 perform the same action for both isDev without VITE_DEV_SERVER_URL and production. This can be simplified.

♻️ Suggested simplification

   // Load Vite dev server in development or static server in production
   if (VITE_DEV_SERVER_URL) {
     state.mainWindow.loadURL(VITE_DEV_SERVER_URL);
-  } else if (isDev) {
-    // Fallback for dev without Vite server URL
-    state.mainWindow.loadURL(`http://localhost:${state.staticPort}`);
   } else {
+    // Fallback for dev without Vite server URL, or production
     state.mainWindow.loadURL(`http://localhost:${state.staticPort}`);
   }